Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exercise editor: support updates #73

Merged
merged 2 commits into from
Feb 2, 2018
Merged

Conversation

mb21
Copy link
Collaborator

@mb21 mb21 commented Jan 23, 2018

see #70

Notes

  • enables methodOverride middleware
  • Yesod.Helpers.Crud could come in handy for such use cases, though I haven't looked at it closely, since there was existing code for create and read in this case.
  • adds a column to the DB.

Questions

  • visually, the pages look quite hideous... is that okay for the admin pages, or...?
  • ExerciseSettings with key and value columns doesn't appear to be needed since we have the onSubmitMsg and canAddDeleteGeom columns (added by you), as well as the canEditProperties column (added in this pull).

@achirkin
Copy link
Owner

visually, the pages look quite hideous... is that okay for the admin pages, or...?

That's true :-D It would be nice to make it more usable, but only if we have time for that, since it is only visible to a few admins.

ExerciseSettings with key and value columns doesn't appear to be needed since we have the onSubmitMsg and canAddDeleteGeom columns (added by you), as well as the canEditProperties column (added in this pull).

Yeah.. I added it only because I did not have time for a proper solution :) So I still think, it makes sense to put all settings into another table. Let's keep it in mind, but postpone for a while

@mb21
Copy link
Collaborator Author

mb21 commented Jan 24, 2018

okay, then this should be ready to merge.. or should I add some CSS today?

@achirkin achirkin merged commit f9cbcb6 into achirkin:reflex Feb 2, 2018
@mb21 mb21 deleted the exercise-editor branch February 4, 2018 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants